Repository navigation
fix(frontend): send attached files from the expert side-panel chat and show the message at once (SECRT-2956 1/3) [1/3] - #15582
Conversation
The drawer's onSend ignores the files and workspace files ChatInput hands it, and draws nothing until the session exists. These tests send a local file in a new thread, a workspace file in an existing one, a file while the expert is still replying, and a file queued behind the kickoff, and expect the message on screen at once and file_ids on /stream. They fail until the fix lands. Co-authored-by: Claude Opus 5.5 (Claude Code) <noreply@anthropic.com>
…SendMessage The upload, placeholder and send steps lived inside the /copilot hook, so the expert side panel could not reuse them and sent text only. They now live in helpers/sendWithAttachments.ts as one OutgoingMessage type and a sendWithAttachments() both surfaces can call, with an optional waitUntilReady step between the uploads and the send. The settle waiters in useCopilotPage move to useSettleGate so the panel can wait on its own reply the same way. No behaviour change on /copilot. Co-authored-by: Claude Opus 5.5 (Claude Code) <noreply@anthropic.com>
…d show the message at once The side panel's onSend took only the text, so files and workspace files attached in it never reached the expert, and nothing was drawn until the session existed, which left the empty state up for seconds after Send. Every send path in the panel now goes through sendWithAttachments, so local files upload into the session and go out as file_ids with the message, in new and existing threads and behind the kickoff. The message shows from the first render after Send: held in the panel until a session exists, then as the session's upload placeholder. A follow-up with files sent while the expert is replying uploads at once and goes out as the next turn when the reply ends, because the pending buffer would store its files without their names. If the user starts a new task before it goes out, its text comes back to the composer. The composer now gets the expert's id and name, as on /copilot, so the workspace picker offers that expert's files. Co-authored-by: Claude Opus 5.5 (Claude Code) <noreply@anthropic.com>
…replies The composer closed the attach menu, paste and workspace mentions for the whole reply, and its Queue button only carried text, so a file could not be sent mid-reply even though the drawer now holds such a message and sends it as the next turn. ChatInput takes `canAttachWhileStreaming`. With it, attaching stays open during a reply, and Queue message (like Enter) sends a message that has attachments through `onSend`. Only the side panel sets it: /copilot still refuses to queue files. Co-authored-by: Claude Opus 5.5 (Claude Code) <noreply@anthropic.com>
…nd through a reconnect Two ways a message with files could go wrong after Send: - It was sent with whatever `sendMessage` the panel held when the uploads finished. After New task, that belongs to the new thread, so the old task's text and files were posted to it. A send now checks that its thread is still on screen; if not, the text goes back to the composer. - A follow-up held until the reply ended was released on `status` `ready`/`error` alone. The SDK sets `status` on React's sync lane, but the stream's reconnect and post-finish probe flags render one pass later, so the send went out while the stream was reconnecting. The stream dropped it and the bubble disappeared. The panel now waits for `isReconnecting` and `isFinishProbing` too, and confirms the turn is still over once those later updates have rendered (`useNextCommit`). Tests now attach through the visible menu and send with the Queue button and Enter. They also cover a workspace file picked mid-reply, an upload that outlives its thread, and a connection dropped while a file is held. Co-authored-by: Claude Opus 5.5 (Claude Code) <noreply@anthropic.com>
…anel closes, and over a new draft Three ways the expert side panel still loses or locks a message with files: - a file sent while the connection is being restored is cleared from the screen and never sent; - closing the panel while a file uploads leaves the thread's composer locked when it is opened again, with or without a reply in flight; - a message sent back to the composer after New task replaces what the user has typed in the new task. Co-authored-by: Claude Opus 5.5 (Claude Code) <noreply@anthropic.com>
…nel and a new draft Every message with files now goes out through one send that waits until the reply in flight has ended. Before, that wait only applied when the SDK said a reply was streaming. A message sent while the connection was being restored, or right after a kickoff reply, skipped it, and the stream then refused the send without telling its caller. The settle gate and the commit barrier return false once their host has unmounted. An upload that finished after the panel closed used to wait on them forever, so the thread's placeholder stayed and its composer stayed locked. A message the panel hands back goes into a new recovered-draft slot. The composer adds it to what the user has typed instead of replacing it. Guided prompts keep the replace behaviour of `initialPrompt`. Co-authored-by: Claude Opus 5.5 (Claude Code) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📜 Recent review details
WalkthroughCopilot and Expert Chat now share attachment-aware message handling. Expert Chat can hold attachment-bearing messages until the current reply ends, including during reconnection, and display pending sends. The changes also add upload validation, failed-draft recovery, and attachment-flow tests. ChangesAttachment-aware chat
Priority: ⬇️ Low Estimated code review effort: 4 (Complex) | ~45 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant ChatInput
participant useExpertChatDrawer
participant useExpertChatSend
participant sendWithAttachments
participant useCopilotStream
ChatInput->>useExpertChatDrawer: Submit outgoing message
useExpertChatDrawer->>useExpertChatSend: Send message with attachments
useExpertChatSend->>useExpertChatSend: Wait for reply settlement
useExpertChatSend->>sendWithAttachments: Upload and prepare attachments
sendWithAttachments->>useCopilotStream: Send message parts
Merge Risk: ⚪ Minimal · up to
|
|
/review |
|
🤖 Review of |
…956-expert-side-panel-chat-drops-attached-files-message-should-p1 Resolve the ChatInput.tsx conflict by keeping both props: this PR's canAttachWhileStreaming and dev's atReset. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
🔍 PR Overlap DetectionThis check compares your PR against all other open PRs targeting the same branch to detect potential merge conflicts early. 🔴 Merge Conflicts DetectedThe following PRs have been tested and will have merge conflicts if merged after this PR. Consider coordinating with the authors.
🟢 Low Risk — File Overlap OnlyThese PRs touch the same files but different sections (click to expand)
Summary: 3 conflict(s), 0 medium risk, 5 low risk (out of 8 PRs with file overlap) Auto-generated on push. Ignores: |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev #15582 +/- ##
========================================
Coverage 86.87% 86.87%
========================================
Files 4947 4951 +4
Lines 378328 378471 +143
Branches 33657 33689 +32
========================================
+ Hits 328656 328814 +158
+ Misses 44196 44179 -17
- Partials 5476 5478 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…st one The duplicate-send guard compared only text, so a held side-panel follow-up with a file and the same caption as the previous message was dropped as a resend. The guard now also compares the attachments. Co-authored-by: Claude Opus 5.5 (Claude Code) <noreply@anthropic.com>
…pped Stop ended the reply and the held message with files went out right after, while the backend discards buffered text follow-ups on cancel. Now Stop cancels it, puts its text back in the composer and says why. Co-authored-by: Claude Opus 5.5 (Claude Code) <noreply@anthropic.com>
There was a problem hiding this comment.
✅ Verdict: Approved — No findings require changes before approval; this review counts as an approval.
Well-structured frontend refactor extracting the upload-then-send path into a shared helper and wiring attached files through the expert side-panel chat; no reachable defect found and the concurrency-heavy paths are covered by passing integration tests. Required CI is still pending and the branch currently conflicts with its base, which must be resolved before merge.
Risk level: low | Human review: not required | Duration: 369s | Reviewed: b9cf1507
Findings: 🔴 0 blockers | 🟠 2 should fix | 🟡 1 nice to have | 🔵 1 nit
Optional advice — does not block approval
Follow-ups
- 🟠
autogpt_platform/frontend/src/app/(platform)/team/components/ExpertChatDrawer/useExpertChatDrawer.ts:303Stabilize send/release so drawer effects fire on sessionId, not every render — This effect and the kickoff-dequeue effect at line 249 depend onsend, which useExpertChatSend recreates every render, so both re-run every render. They avoid duplicate sends only because pendingPromptRef/kickoffAttemptRef are nulled synchronously inside each effect; a future edit to that guard would turn the benign re-run into a duplicate send.
Suggestion: Stabilize send/release behind a ref (as useSendMessage.ts does with dispatchRef) so the effects trigger on sessionId change only. - 🟠
autogpt_platform/frontend/src/app/(platform)/team/components/ExpertChatDrawer/useExpertChatSend.ts:128Scope recoveredDraft so a dropped drawer message can't reappear in /copilot — recoverUnsentMessage/returnToComposer write to the global useCopilotUIStore. If the drawer unmounts before useChatInput consumes recoveredDraft, the text persists and is appended into the next-mounted consumer (e.g. /copilot). No data loss since it appends, and the window is narrow.
Suggestion: Scope the recovery slot per surface or clear recoveredDraft on drawer unmount.
Optional advice — does not block approval
🟡 Nice to Have
- 🔵
autogpt_platform/frontend/src/app/(platform)/team/components/ExpertChatDrawer/ExpertChatDrawer.tsx:190Keep composer usable while a mid-reply attachment is merely held — isUploadingFiles is bound to !!pendingSend, which stays true for the whole reply while a follow-up attachment is held, disabling the textarea the entire time rather than only during the upload. Explicitly deferred to PR 2.
Suggestion: Distinguish 'actively uploading' from 'held waiting for reply to end'; acceptable to defer with the PR 2 per-file-status work.
Optional advice — does not block approval
🔵 Nits
- 🔵
autogpt_platform/frontend/src/app/(platform)/team/components/ExpertChatDrawer/__tests__/attachments.test.tsx:254Reset useCopilotUIStore in attachments test beforeEach — beforeEach resets the stream store and localStorage but not useCopilotUIStore, which now holds mutable cross-test state (recoveredDraft, initialPrompt). No leak today, but a future test could become order-dependent.
Suggestion: Add useCopilotUIStore.setState({ recoveredDraft: null, initialPrompt: null }) to beforeEach.
GitHub CI on reviewed head: failure
Validation and specialist details
Specialist Reports
| Specialist | Status | Summary |
|---|---|---|
| security | ✅ PASS | Frontend-only chat composer refactor with no new security surface; workspace-file scoping is correctly tightened via expertId and file-size/count validation is preserved. |
| architect | ✅ PASS | Well-structured extraction that routes drawer attachment sends through the shared upload-then-send path with carefully-correct settle/commit timing and thread-identity guards; no blocking defects, one low maintainability note. |
| performance | ✅ PASS | Client-side upload/send coordination with no reachable performance, memory, or scalability defects; uploads are parallelized and all async-gate waiters are cleaned up on settle and unmount. |
| testing | ✅ PASS | Comprehensive, assertion-strong integration tests directly verify the attachment-send fix and every hold/recovery path; no blocking test defects found. |
| quality | ✅ PASS | Clean extraction of the upload-then-send path into shared helpers plus a well-tested fix that sends side-panel attachments and renders the user's message on the first frame; no concrete defects found. |
| product | ✅ PASS | Attachment sending from the expert side-panel is implemented correctly across all send states with the message shown on the first frame; copy rules respected and edge cases covered by tests, with one advisory composer-disabled UX limitation. |
| discussion | ✅ PASS | No open human-reviewer threads; required checks pending, but the branch has a merge conflict with its base that must be resolved. |
| ui-reviewer (local) | ✅ PASS | Frontend fix wiring attached files through the expert side-panel chat is sound and well-covered; targeted drawer/ChatInput/useSendMessage test suites pass (99 tests) and prop/store contracts verify, with only a low-severity cross-surface recoveredDraft advisory. |
| ui-reviewer (hosted) | ✅ PASS | The drawer now forwards attached files/workspace files to sendWithAttachments so the expert side panel uploads and sends file_ids; logic is sound and the new plus existing copilot integration suites pass (184 tests) with no regressions. |
EvidenceVerdict: PASS at
Requests the browser recorded: Also: 277/277 targeted tests across 15 files (drawer Notes: while a held follow-up waits for the reply to end, its bubble still says "Uploading 1 file…" after the upload has finished. PR 2 replaces that flag with a status for each file. The "Connection lost / Reconnecting…" toast in some shots comes from the stream's existing resume logic, which this PR does not change. The held files went out in every run. |
|
/review |
|
🤖 Review of |
There was a problem hiding this comment.
✅ Verdict: Approved — No findings require changes before approval; this review counts as an approval.
A disciplined frontend refactor that extracts shared upload-then-send/settle-gate logic and wires attachments through the expert side-panel chat with a flash-free held-message handoff. Every send path and the duplicate-suppression change were traced and are covered by strong, deterministic integration tests; no reachable defect found and CI is green on the reviewed head.
Risk level: low | Human review: not required | Duration: 480s | Reviewed: 2080f378
Findings: 🔴 0 blockers | 🟠 1 should fix | 🟡 2 nice to have | 🔵 0 nits
Optional advice — does not block approval
Follow-ups
- 🟠
autogpt_platform/frontend/src/app/(platform)/team/components/ExpertChatDrawer/ExpertChatDrawer.tsx:190Relax composer lock while a held follow-up waits on the reply — isUploadingFiles={!!pendingSend} disables the textarea and attach menu for the entire time a follow-up is held, including the full in-flight reply it waits on, not just the byte upload. Stop stays enabled so the user is not trapped.
Suggestion: In the planned PR 2, gate the textarea lock on actual upload-in-progress rather than on any pendingSend.
Optional advice — does not block approval
🟡 Nice to Have
- 🔵
autogpt_platform/frontend/src/app/(platform)/copilot/helpers/sendWithAttachments.ts:171Collapse the stacked upload-failure toasts — A single failed local upload can surface up to three stacked destructive toasts (per-file, aggregate, and send-failure). Inherited from the existing /copilot path, not introduced here.
Suggestion: Collapse the cascade into one message in a follow-up. - 🔵
autogpt_platform/frontend/src/app/(platform)/copilot/helpers.ts:318Confirm duplicate-suppression asymmetry is intended — Resending the same caption with fewer/no attachments than the previous message is no longer suppressed. Defensible as a materially different message but a behavior change beyond the stated fix.
Suggestion: Confirm the asymmetry is intended; no code change required.
GitHub CI on reviewed head: success
Validation and specialist details
Specialist Reports
| Specialist | Status | Summary |
|---|---|---|
| security | ✅ PASS | Frontend chat attachment refactor with server-enforced file scope/limits and an explicit same-session guard against cross-thread send; no concrete security defects found. |
| architect | ✅ PASS | Well-structured refactor that extracts shared upload-then-send/settle-gate logic and wires it into the expert drawer with correct hold/release, kickoff, reconnect and Stop handling; no reachable defect found and coupling stays unidirectional. |
| performance | ✅ PASS | Frontend attachment-send extraction and drawer hold/settle logic are carefully guarded against the cross-thread and lifecycle races they introduce; no concrete performance or correctness defects found. |
| testing | ✅ PASS | The new MSW integration suite and targeted helper/ChatInput tests cover every fixed behavior with strong, deterministic, regression-catching assertions; no concrete test defect found. |
| quality | ✅ PASS | Well-structured refactor that correctly fixes attachment sending in the expert side panel with no concrete defects found; the shared /copilot path is behavior-preserving and copy rules are respected. |
| product | ✅ PASS | Attachment send paths in the expert side panel are correctly unified, the held-message handoff is flash-free, and error/Stop/reconnect edge cases are handled; no concrete blocker found. |
| discussion | ✅ PASS | CI is green on the current head and there are no open human-reviewer requests. |
| ui-reviewer (local) | ✅ PASS | Source review of the expert side-panel attachment send/hold logic found no reachable defect; every send path (new thread, existing thread, mid-reply, kickoff, stop, new-task, reconnect) and the duplicate-suppression change are coherent and match the added tests. |
| ui-reviewer (hosted) | ✅ PASS | Frontend-only fix that correctly uploads and sends side-panel chat attachments; targeted and full copilot test suites pass (3918+164) with no regression and no reachable defect found. |
|
🤖 @Abhi1992002, Are you already on it as part of SECRT-2956? If not, we can take it; say so here and we will open a PR, so we don't both fix it. Update 17:2xZ: it ejected #15643 from the queue as well (merge-group run 39290586468), the third ejection, so we would like it fixed soon either way. |
Dev moved the upload-and-send logic into helpers/sendWithAttachments.ts (#15582), so the reply timing now rides on OutgoingMessage there instead of an extra dispatch argument. Dev's new stable-onsend test (#15679) expects the send's reply-timing start as its fifth argument. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…resets, a refused first send and partial codecov verdicts (Significant-Gravitas#15759) This PR removes the causes of seven recurring e2e and CI flakes. Each backend service exempts its startup heap from full garbage collections, and the scheduler and copilot executor load the block registry at startup. RPC servers and the Docker frontend keep idle connections longer than their clients do. A chat message sent while the page is still loading its AI connections waits for them instead of being refused. Codecov posts a backend or frontend verdict only once every shard has uploaded, and the e2e stack's Redis cluster is created only once every shard answers. ### Why / What / How **Why:** each of these failed tests or blocked merges for reasons unrelated to the change under test. On 19 Oct, 6 of 17 Full-stack runs failed between 22:49Z and 23:26Z, and Significant-Gravitas#15697 was ejected from the merge queue. The e2e stall is tracked internally as SECRT-2966. Resolves Significant-Gravitas#15754 Resolves Significant-Gravitas#15700 Fixes AUTOGPT-SERVER-A6D Before and after, per class. "Before" is 80 Full-stack runs on dev, merge-queue and PR heads on 18–19 Oct. "After" is 8 runs dispatched on `336ef7cadf`, every fix here except the frontend send fix, whose before-rate (2 in 80) is too low to show in 8 runs; its test fails without it. | Class | Before | After | | ------------------------------------------------------------ | ----------------------------------------------- | --------------------------------------------------------------------------------------------------- | | Failed e2e jobs | smoke 7 of 80, journeys 10 of 80 | smoke 0 of 8, journeys 0 of 8 | | REST event-loop step of 2 s or more, per smoke job | 31 of 80 (longest 14.87 s) | 0 of 8 (longest 0.37 s) | | Copilot executor loads the block registry during turns | 16 of 16 journeys jobs sampled | 0 of 8 | | RPC `ReadError` or "Failed to process" in the REST log | 24 of 80 journeys jobs (62 lines) | 0 of 8 journeys jobs, 0 of 8 smoke | | `socket hang up` on a reused connection | 2 tests (runs 39337657617, 39383564514) | 0 | | Stack never starts: `redis-0` unhealthy | 2 of 426 e2e jobs since 18 Oct (both on 20 Oct) | reproduced locally; 0 in the runs below | | Final backend codecov verdict posted before 12 of 12 uploads | 6 of 12 commits | 0 of 4 heads from part of the shards; 1 of 4 from the parent's carried-forward coverage (see below) | 1. **Full garbage collections froze services for seconds.** A full (generation 2) collection walks every tracked object while holding the GIL, so every thread in the process stops until it finishes. A service that has loaded the block registry holds 1.6–2.2M tracked objects, nearly all long-lived startup state (block classes and their schemas). Locally one full collection over that heap takes 559–776 ms, which is the pause a production process pays each time; this is estimated from a local measurement, and Sentry and the production logs were not checked for it. On the e2e runner the heap is partly swapped out, so the collector pages it back in while every thread waits. The REST server's event loop then froze for 4–15 s, every `/api/proxy` call made meanwhile waited as long, and a test waiting on the UI timed out: the Library `:390`, `:497` and `:541` failures, and `publish-happy-path.spec.ts:9`, whose search navigation waited on six `/api/proxy` calls the frozen server held for 5.6 s. 2. **The copilot executor's first turns were slow.** It loaded the block registry lazily, inside the first turns that needed it, and the heap that load grew triggered a full collection: one took 6,146 ms during the journeys' first turns (run 39387674137). The follow-up and reconnect journeys then missed their 15 s wait for the answer to start streaming, the failure behind 6 of the 13 failed journey tests. 3. **RPC calls failed with `httpx.ReadError`.** Every `AppService`'s uvicorn closed an idle keep-alive connection after its default 5 s, while the RPC clients kept it pooled for 30 s. A call written into a connection as the service closes it is reset. In the journeys, the REST server's call to the scheduler failed that way and the route answered 500 "Failed to process … /schedules": 4 of the 13 failed journey tests, 4 of 5 runs on Significant-Gravitas#15616, and the Significant-Gravitas#15697 ejection. The scheduler made it likely: it loaded every block on its first new schedule, 12 s on the runner, and froze for 6–7 s of it. In production this is AUTOGPT-SERVER-A6D: the executor's `update_graph_execution_stats` call to the database manager failed with `ReadError` 16 times on 9 Oct, during one user's 20,953-node run, and its retry gave up. Idling alone is harmless, because httpcore drops a connection the service has already closed. The failure needs the close and the call to coincide, and a freeze makes every pooled connection that receives a call meanwhile hit the boundary at once. Reproduced against a test `AppService` in a child process, called through the real `get_service_client`: | Trial | Calls | Before: ReadError | After: ReadError | | ------------------------------------------------------------------- | ----- | ----------------- | ---------------- | | idle 6, 10 or 20 s, no stall | 60 | 0 | 0 | | reuse within ±50 ms of the 5 s keep-alive | 102 | 11 | 0 | | the caller's loop blocked 0.4–1.5 s just before the boundary | 60 | 4 | 0 | | the service's loop blocked 7 s inside a request | 10 | 0 | 0 | | the service's GIL held 2–6 s by another thread, across the boundary | 45 | 42 | 0 | 4. **The Docker frontend reset reused connections.** Node closes an idle keep-alive socket after 5 s plus a 1 s buffer. Playwright's request client keeps pooled sockets with no expiry of its own and does not retry, so a test request on a socket idle for about 6 s met the server's FIN and failed with `socket hang up`: after 5.99 s idle in `copilot-build-agent-journey.spec.ts:30` (run 39337657617), and after 5.93 s in `copilot-markdown-happy-path.spec.ts:38` (run 39383564514). 5. **A first message sent during page load was refused.** Starting a chat refused a send made before the AI connection list had loaded: "AI connections are still loading. Wait a moment and try again". The composer then put the text back. On a freshly loaded page that list took 1.8 s on the runner, and `copilot-build-agent-journey.spec.ts:30` pressed Enter about 100 ms after that request finished, before the page had rendered with the list. The message was never sent, and the test waited 60 s for a stream (runs 39339438495 and 39337526897: 2 of the 13 failed journey tests). A user who sends that early gets the same refusal and has to send again. 6. **The e2e stack sometimes never started.** `redis-init` creates the Redis cluster once, as soon as the three shard containers have started, but a started shard can take a second to accept connections. In runs 39396005336 and 39397892921 it reached `redis-0` 0.4–0.6 s too early ("Could not connect to Redis at redis-0:17000: Connection refused") and exited. `redis-0`'s healthcheck needs `cluster_state:ok`, so it went unhealthy 5 min later and the job failed before any test ran. 7. **Codecov posted verdicts from part of the shards.** It recomputed its statuses after each upload. On 6 of 12 recent commits, its last `codecov/project/platform-backend` verdict came from 8–11 of the 12 backend shard uploads, and nothing was re-posted after the rest arrived. `Check PR Status` fails on a red one, which only a re-run clears. **What:** - `freeze_startup_heap()` runs once a service has started. It runs at the end of the REST server's lifespan, after every `AppService`'s lifespan (scheduler, database manager, notifications, platform linking, the copilot chat bridge, the swap service, the batch executor), and after the graph executor's and the copilot executor's block preload. Later full collections then walk only what was allocated after startup. - The scheduler and the copilot executor load the block registry before they serve or take turns, so the freeze covers the blocks too. - RPC services keep an idle connection for 60 s, and clients retire theirs at 30 s. - The Docker frontend keeps idle connections for 65 s. - Starting a chat waits for the AI connection list when it is still loading, and refuses only if the list never arrives. - `redis-init` waits up to 30 s for each shard to answer `PING` before creating the cluster, as backend CI's cluster setup already does. - Codecov waits for 12 `platform-backend` uploads and 4 `platform-frontend` uploads before posting those flags' statuses. **How:** the helper calls `gc.collect()` and then `gc.freeze()`, and logs how many objects it froze and how long that took. It collects first, so garbage still in the heap is freed rather than frozen in place for good. For `AppService`, `build_fastapi_app` wraps the service's lifespan and freezes after the subclass's whole startup has run. `AppService` passes `timeout_keep_alive` to uvicorn, and both RPC clients derive `keepalive_expiry` from it as half of it. The 30 s margin also covers a caller whose own event loop stalls between taking a connection from the pool and writing to it. Next's standalone server reads `KEEP_ALIVE_TIMEOUT`. `createSession` awaits `refetch({ cancelRefetch: false })` on the transports query, which joins the request already in flight rather than starting a second one. Codecov's per-flag `after_n_builds` counts each flag's own uploads, so a PR that runs only the frontend suite doesn't wait for backend shards. Borderline decisions: - Every `AppService` freezes its heap after startup, including services that never load blocks, because the call sits in the base class's lifespan wrapper (`_serving_lifespan`). The freeze costs one collection at startup: 0.8–3.0 s on the runner. - The websocket server has no freeze, because it doesn't load the block registry. - The RPC service's keep-alive was raised, rather than the client's expiry lowered. An expiry under 5 s would reconnect on nearly every scheduler call, and would leave under a second of margin against a stall in the caller. - There is no unit test for the scheduler's preload. The only assertion available would pin a call inside a heavily mocked `run_service`. Its startup log line ("Loaded N blocks in …") shows it instead. - The frontend's keep-alive moves the race with a client that never expires its sockets from about 6 s of idleness to about 66 s; it cannot remove it. - A send made while the connection list loads waits as long as that request takes, with no deadline of its own. - When another flag uploads before the first backend shard, Codecov counts the parent's carried-forward backend uploads toward the 12, and posts `codecov/project/platform-backend` from them. On `c08de8480f` it posted at the first frontend upload and was not re-posted after the 12th backend upload, while `codecov/patch/platform-backend` waited for all 12. That verdict is the parent's coverage, so it cannot be a partial-shard red, but it does not reflect this PR's backend coverage either. On the other three heads checked, both backend statuses waited for 12 of 12. - A PR that runs no backend tests may get no `platform-backend` codecov status, if Codecov does not count carried-forward uploads toward `after_n_builds`; that was not tested here. It blocks nothing, because `Check PR Status` is the only required check. Known flakes this PR does not touch: the credits rate-limit clock, owned by Significant-Gravitas#15452, and the ExpertChatDrawer attachments test, Significant-Gravitas#15582. Made diagnosable, not fixed: `e2e_redis_restart_test.py::test_subscriber_survives_shard_restart` failed in 1 of 415 `data`-shard jobs (run 39391205056) with "port 27111 never opened within 60s" and nothing else to go on. Its timeout now appends the cluster's `docker ps -a` and the shard's last 50 log lines. ### User-visible change A message typed and sent while the chat page is still loading goes out once the page has loaded its AI connections, instead of being refused with "AI connections are still loading". If the connections never load, the send is refused with "Could not check AI connections", as a failed load already was. ### Changes 🏗️ - `backend/util/process.py`: `freeze_startup_heap()`. - `backend/util/service.py`: `AppService.build_fastapi_app` wraps the lifespan so the freeze runs after the subclass's startup. `rpc_server_keepalive_seconds = 60` goes to uvicorn's `timeout_keep_alive`, and `rpc_client_keepalive_expiry_seconds`, half of it, replaces both hard-coded `keepalive_expiry=30.0`. - `backend/api/rest_api.py`: the REST lifespan freezes after the catalog load. - `backend/executor/manager.py`: the graph executor freezes after its block preload, before its worker threads start. - `backend/executor/scheduler.py`: `run_service` loads the block registry before serving, and logs how long that took. - `backend/copilot/executor/manager.py`: the copilot executor loads the block registry and freezes before it takes turns. - `frontend/src/app/(platform)/copilot/useChatSession.ts`: `createSession` waits for a connection list still loading. - `docker-compose.platform.yml`: `KEEP_ALIVE_TIMEOUT: "65000"` on the frontend, and `redis-init` waits for every shard's `PONG`. - `codecov.yml`: `after_n_builds` on the `platform-backend` (12) and `platform-frontend` (4) flags. - `backend/data/e2e_redis_restart_test.py`: the port-wait timeout reports the shard's container state and log. - Tests: - the helper freezes the heap, and collects garbage instead of freezing it; - an `AppService` freezes after its own startup; - both executors load the blocks and freeze before their threads start; - a pooled RPC call made while the service's GIL is held past the connection's idle time is answered rather than reset; - a chat started while the connection list loads waits for it and is created, and one whose list never arrives is refused. ### Agents and large language models used - Claude Code with Claude Opus 5.5 ### Checklist 📋 #### For code changes: - [x] I have clearly listed my changes in the PR description - [x] I have made a test plan - [x] I have tested my changes according to the test plan: - [x] Unit tests for the helper and every call site, each shown failing under the mutation it guards - [x] `architecture_test.py`, `test_block.py` and the touched suites, backend and frontend - [x] Full-stack CI dispatched on this branch, failures and loop blocks counted against the 80-run baseline - [x] Reproduced the `ReadError` and the `socket hang up` races, and re-ran them with the fixes **Verified:** On the backend I ran these, all passing: - `process_test.py`, `service_test.py` and `swap_service_test.py`; - both executors' `manager_block_warmup_test.py`, plus `manager_test.py` and `manager_cancel_consumer_test.py`; - `entry_points_test.py`, `capability_tools_test.py` and `architecture_test.py`; - `test_block.py` (3,696 passed, 84 skipped). Each new backend test went red under its mutation: dropping `gc.freeze()` or `gc.collect()`, moving the freeze before the subclass's startup, building the app from `self.lifespan`, and dropping either executor's preload or freeze. The keep-alive test failed 5 of 5 times with `httpx.ReadError` without the server keep-alive, and passes with it. On the frontend, the copilot suite passes (314 files, 4,131 tests), as do `tsc` and `eslint`. The two new `createSession` tests fail against the old hook (2 failed, 9 passed) and pass with the fix. A Node client reusing a socket after 5.990–6.005 s idle failed 4 of 4 times against Node's default keep-alive, and succeeded 4 of 4 times with `KEEP_ALIVE_TIMEOUT=65000`. Codecov's validator accepts `codecov.yml`. With `redis-0` started 3 s late in a throwaway compose project, the old `redis-init` exited 1 with "Connection refused" and no cluster formed. The new one waited and formed it (`cluster_state:ok`), and it still forms one in 2 s with no delay. The Full-stack measurements are in the table above. Two more runs on `4eadfd936e`, which has the frontend send fix, repeated the journeys three times, and both legs passed (the build-agent journey 6 of 6; runs 39393594364 and 39393597786). Run ids and logs are in the comments below. #### For configuration changes: - [x] `.env.default` is updated or already compatible with my changes - [x] `docker-compose.yml` is updated or already compatible with my changes - [x] I have included a list of my configuration changes in the PR description (under **Changes**) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…hment tests (Significant-Gravitas#15792) ### Why / What / How **Why:** "sends a workspace file picked mid-reply as the next turn" in the ExpertChatDrawer attachments tests (added in Significant-Gravitas#15582) has failed often in CI and the merge queue since the React 19 upgrade (Significant-Gravitas#15746). It knocks unrelated PRs out of the queue. The failure is `Unable to find role="button" and name /brief\.pdf/` in `pickWorkspaceFile()`, and sometimes the test hits its 5 s timeout instead. **What:** `pickWorkspaceFile()` now finds the picker dialog first and looks for the file button inside it, not across the whole page. **How (root cause):** This is not a logic race. The picker opens and the file list request goes out; the list just hasn't rendered when findBy gives up. The time goes into the query itself. A page-wide `*ByRole` check reads the computed style of every element, and happy-dom recomputes those styles after every DOM change. Measured locally, the first page-wide check after the picker opens takes 100-200 ms, while the same check scoped to the dialog takes about 15 ms. A full style pass over the page takes 240-640 ms. These checks run on the main thread, so they also hold back the mock response and the render the test is waiting for. On a busy CI runner that uses up findBy's default 1 s. "sends a workspace file picked in an existing thread" uses the same helper and failed the same way locally. ### Changes 🏗️ - `attachments.test.tsx`: `pickWorkspaceFile()` gets the dialog "Use a file from your workspace" and runs the `brief.pdf` and "Add 1 file" queries inside it with `within()`. No timeouts were raised and no sleeps were added. No product code changed. ### Agents and large language models used - Claude Code — Claude Opus 5.5 ### Checklist 📋 #### For code changes: - [x] I have clearly listed my changes in the PR description - [x] I have made a test plan - [x] I have tested my changes according to the test plan: - [x] Ran the whole test file 112 times before the fix, 14 runs in parallel on 14 cores (part of them with 8 extra CPU burners): **5 failures** (all at the `brief.pdf` step, in both tests that use the helper, plus one 5 s timeout) - [x] Same 112 runs under the same load after the fix: **0 failures** - [x] Whole `ExpertChatDrawer` test folder passes (28 tests) - [x] `pnpm format`, `pnpm lint`, `pnpm types` pass 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>















































Part of SECRT-2956
https://linear.app/autogpt/issue/SECRT-2956
This is PR 1 of 3. It fixes the bug. PR 2 adds per-file upload status with Retry, and PR 3 adds the new attachment cards.
Why / What / How
Why. In the Team page's expert side panel, a file attached to a message never reached the expert. The composer showed the file and cleared it on Send, but the drawer's
onSend(message: string)ignored thefilesandworkspaceFilesarguments thatChatInputpasses. Nothing was uploaded and nofile_idswent to/stream. In a new thread the panel also stayed on the "What can I do for you?" empty state until session creation returned (about 3 s), then showed the message as text only.What. The side panel now uploads local files and workspace files and sends them with the message, in a new thread, in an existing thread, while the expert is still replying, and behind the expert's kickoff turn. The user's message and its files show on the first frame after Send, before any network call returns.
How. The upload-then-send code from
/copilot'suseSendMessagemoves into a plain module,sendWithAttachments, and both surfaces call it. On/copilotthe code moves without changes, and its tests pass unedited. The drawer keeps its own session and kickoff logic and holds the message locally until a session exists, so it never uses/copilot's global first-send slot. A follow-up with files sent mid-reply does not go through/messages/pending, because that buffer stores files without names and the transcript would drop them. The drawer holds it instead: the bubble shows and the upload starts right away, then the message goes out as the next turn once the reply has ended. Text-only follow-ups still use the buffer.Changes 🏗️
copilot/helpers/sendWithAttachments.ts(new):OutgoingMessage,validateAttachments,describePendingUploadandsendWithAttachments({ sessionId, message, sendMessage, waitUntilReady? }), moved out ofuseSendMessage.ts.waitUntilReadyruns after the uploads and before the send; returningfalsedrops the placeholder.copilot/useSendMessage.ts: calls the helper. No behaviour change on/copilot.copilot/useSettleGate.ts(new): the "wait until the reply has ended" gate, taken out ofuseCopilotPage.ts, which now uses it. After unmount it resolvesfalserather than hanging.team/components/ExpertChatDrawer/useExpertChatSend.ts(new): holds the user's message on screen until it can be sent, and sends every message with attachments through one gatedsend(). Before sending, it checks that the message's thread is still the one on screen and that the reply has fully ended, including any reconnect or finish probe.team/components/ExpertChatDrawer/useNextCommit.ts(new): waits one React commit, so the gate does not open on anerrorstatus the render before the stream marks itself as reconnecting.useExpertChatDrawer.ts:onSend(text, files?, workspaceFiles?)builds anOutgoingMessage. Every send path (first send after create, kickoff dequeue, the not-active fallback, a plain send) goes throughsend(). Thread resets clear the held message.ExpertChatDrawer.tsx: renders the transcript when there is a session or a pending message, keeps onboarding hidden while a message is pending, and passesisUploadingFiles,expertIdandexpertNametoChatInput. WithoutexpertId, the backend rejects workspace files outside the expert's scope.ChatInput: new opt-incanAttachWhileStreamingprop, set only by the drawer, so files can be attached and queued while the expert replies./copilot, the builder chat and memory chat keep today's behaviour.copilot/store.tsanduseChatInput.ts: arecoveredDraftslot. If a held message can't go out (for example after the user starts a new task during its upload), its text is added to the composer next to any new draft instead of replacing it, with a "Message not sent" toast.copilot/helpers.ts:getSendSuppressionReasonnow calls a send a duplicate only when both the text and the attachments match the last user message. Before, a new file sent with the same caption as the last message was dropped, in the drawer and on/copilottoo.useExpertChatSendcounts Stops, and a held send that sees a Stop after it was queued does not go out. Its text goes back to the composer with a "Message not sent" toast. This matches the backend, which throws away buffered follow-ups when a reply is cancelled.ExpertChatDrawer/__tests__/attachments.test.tsx(new, MSW). It covers a new thread, an existing thread with a workspace file, mid-reply hold, kickoff hold, a dropped connection, a send during a reconnect, closing the panel during an upload, and a new task during an upload. Plus aChatInputtest for queuing files during a reply.Known limits, as the plan expected:
Evidence 📸
Verdict: PASS at
2080f3787(round 5). This replaces the earlier evidence taken atb9cf15078. Since then the branch mergeddev, which changed the shared composer, and added the duplicate-send fix and the Stop fix, so every row was proven again on the running app (Docker deps, native backend,next dev). Synthetic users and data only, reduced motion on, 1440×1000 and 390×844. No model key was available, so a scripted provider answered the turns. There are no "before" shots because there was no dev URL to capture from./streamcarriesfile_ids(create → upload?session_id=→/stream)file_ids/messages/pendingcallfile_idsfile_ids/copilotnew chat still shows the "Uploading 1 file…" bubble/copilotexisting chat still sends a file withfile_ids2080f3787): Stop while a follow-up with a file is held. The bubble goes, the text is back in the composer with a "Message not sent" toast, and no/streamwith files goes outname (type, size), file_id=…).Requests the browser recorded:
Also: 277/277 targeted tests across 15 files (drawer
__tests__,ChatInput__tests__,useSendMessage,CopilotPage,helpers.test).Notes: while a held follow-up waits for the reply to end, its bubble still says "Uploading 1 file…" after the upload has finished. PR 2 replaces that flag with a status for each file. The "Connection lost / Reconnecting…" toast in some shots comes from the stream's existing resume logic, which this PR does not change. The held files went out in every run.
Agents and large language models used
Checklist 📋
For code changes:
2080f3787: 277/277 across 15 targeted files in the evidence step (drawer__tests__,ChatInput__tests__,useSendMessage,CopilotPage,helpers.test). Earlier, in review atb9cf15078, 308/308 across 25 files.attachments.test.tsxfailed before the fix and passes after it, and the two drawer tests added for the duplicate-caption and Stop fixes fail on the code before them.pnpm format,pnpm lintandpnpm typesare clean.next dev), 1440 and 390 wide: new thread with a local image. The bubble shows 50 ms after the click (52 ms at 390) while session create is held back, then the upload runs, then/streamwithfile_ids.file_ids./messages/pendingcall, and goes out as the next turn withfile_ids.file_ids./copilotstill sends files in new and existing chats, with the "Uploading 1 file…" bubble.name (type, size), file_id=…).pnpm test:unitand Playwright suites (CI).For configuration changes:
No configuration changes.
.env.defaultis updated or already compatible with my changesdocker-compose.ymlis updated or already compatible with my changes🤖 Generated with Claude Code
Open workspace in Conductor